#14 [feat] Spring Rest Controller Advice 구현 #16
Conversation
2zerozu
requested review from
jinsu4755 and
jiyeoon00
and removed request for
a team
There was a problem hiding this comment.
고생하셨습니다~
기본적으로 body에 불필요한 정보인 http status code 가 들어가는 것을 막고자 success 에 body에 status code 를 제외하는 방식을 사용했는데 해당 경우에 결국 다시 body에 status code가 들어가는 것 같아요... 어떻게 해결하면 좋을지 논의 해보면 좋을 것 같습니다!
또한 패키징 관련이긴한데 개인적으로 controllerExceptionAdvice는 common 이라는 공통의 페키징에서 필요하기보다 발생한 에러를 처리하는 느낌이라 패키징을 모듈로서 나눈다면 controller 에서 이를 처리하는 것이 좋지 않을까? 생각이 듭니다!
또한 dto 패키징 또한 외부에서 따로 관리하기로 하였기 때문에 이 부분이 common 내부에 들어있을 필요는 없을 것 같아요 :)
dto 패키징 정도는 수정하면 좋을 것 같아서 해당 부분 수정되면 바로 approve 하겠습니다!
src/main/java/com/universe/uni/common/ControllerExceptionAdvice.java
Outdated
Show resolved
Hide resolved
src/main/java/com/universe/uni/common/ControllerExceptionAdvice.java
Outdated
Show resolved
Hide resolved
| protected ErrorResponse handleException(final Exception exception) { | ||
| return ErrorResponse.error(ErrorType.INTERNAL_SERVER_ERROR); | ||
| } | ||
|
|
There was a problem hiding this comment.
MissingRequestHeaderException도 잡아서 request header가 없을 때 처리해주면 좋을 것 같아요!
| public UnauthorizedException(ErrorType error) { | ||
| super(error, error.getMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
BadRequestException, ConflictException, NotFoundException, UnauthorizedException 모두 ErrorType을 매개변수로 받아 ApiException에 넘겨주는 똑같은 역할을 하는데, 나눠서 커스텀 예외를 생성하신 이유가 궁금합니다...!
좀 더 명시적으로 어떤 관련 에러인지 보여주기 위함일까요...?
There was a problem hiding this comment.
네!
- 예외 클래스 이름을 통해 예외의 원인을 빠르게 인지하여 가독성을 높임
- ApiException을 통해 구체적인 예외를 처리하기 위해서는 에러 메시지나 에러 타입을 체크하는 추가적인 로직이 필요함 (번거로움)
의 이유로 따로 나눴습니다
common.exception에 각각의 |
✒️ 관련 이슈번호
🔑 Key Changes
📢 To Reviewers